-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update grafana template #300
Conversation
Thanks for fixing this 🚀 |
@ishanjainn Could you review this when you have a moment? It’s critical, as without this, for example, the entire LDAP authentication is not working. |
Interesting, nice. This was already supported using dotted keys: grafana_ini:
auth.anonymous:
enabled: true
auth.basic:
enabled: true
auth.ldap:
enabled: true I believe grafana helm chart (which now uses same syntax) requires the above format. My thinking was to throw error on nesting under |
{% endif %} | ||
{% endfor %} | ||
{% else %} | ||
{{ section }} = {{ items }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or can sectionless keys be set here if item is not mapping
?
They need to be at the top before any sections to be sectionless. I don't suppose the not mapping
items are guaranteed to be iterated first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. Sectionless items like instance_name
and app_mode
will be included in the last else statement. Your last sentence is correct. Currently, if you place them at the top of your YAML file, they will be guaranteed to appear first. However, if you place them at the end, they will not. We should revert this part to the previous if logic. I will open a new MR to address this issue. Thank you for identifying this edge case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR fixes a bug in the
grafana.ini
creation process.The issue appeared after #232, specifically in https://github.com/grafana/grafana-ansible-collection/pull/232/files#diff-5a971151cd3b0d5b4879eba8415e8b2064b062f86a82779fc6fa24a612c389b7
The current impl. does not handle subsections correctly, such as
auth.ldap
, correctly.Here is an example of the current output:
This format is incorrect. It should generate something like: https://github.com/grafana/grafana/blob/main/conf/sample.ini#L637
This PR fixes the issue, generating something like: